Skip to content

Apply lockdown check to pull_request_read get_diff/get_files#2883

Merged
SamMorrowDrums merged 3 commits into
kerobbi/lockdown-shared-enforcementfrom
kerobbi/lockdown-pr-read-paths
Jul 16, 2026
Merged

Apply lockdown check to pull_request_read get_diff/get_files#2883
SamMorrowDrums merged 3 commits into
kerobbi/lockdown-shared-enforcementfrom
kerobbi/lockdown-pr-read-paths

Conversation

@kerobbi

@kerobbi kerobbi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Note

Stacked on #2881.

Summary

Applies the shared lockdown author check to pull_request_read's get_diff/get_files, and fixes an empty-author edge in get_reviews.

Why

Part of https://github.com/github/copilot-mcp-core/issues/1798 - these two read branches didn't apply the lockdown check, and the get_reviews didn't drop reviews with an empty author on an all-empty page.

What changed

  • Add enforcePullRequestLockdown (fetches the PR to resolve the author under lockdown, then applies authorLockdownResult); use it in GetPullRequestDiff and GetPullRequestFiles.
  • GetPullRequestReviews: drop reviews with an empty author login and hoist the filtered-slice assignment out of the loop so an all-empty page returns an empty list.
  • Tests: lockdown allow/deny for diff/files, a PR-fetch-failure case, and an all-empty-author reviews regression.

MCP impact

  • Tool schema or behavior changed - no schema change; under lockdown, diff/files for a PR whose author lacks push access are now withheld (consistent with GetPullRequest).

Prompts tested (tool changes only)

  • "Get the diff for PR X from Y" with lockdown mode enabled/disabled
  • "Get the changed files for PR X from Y" with lockdown mode enabled/disabled

Security / limits

  • Data exposure, filtering, or token/size limits considered - extends existing lockdown filtering and fixes an empty-author edge in get_reviews; no change outside lockdown mode.

Tool renaming

  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed

@kerobbi
kerobbi marked this pull request as ready for review July 14, 2026 12:21
@kerobbi
kerobbi requested a review from a team as a code owner July 14, 2026 12:21
Copilot AI review requested due to automatic review settings July 14, 2026 12:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends lockdown filtering to additional pull-request read operations and fixes empty-author review filtering.

Changes:

  • Enforces PR-author lockdown checks for diffs, files, and check runs.
  • Correctly removes reviews with empty author logins.
  • Adds lockdown and regression tests.

Exported function signature changes introduce Go API compatibility issues.

Show a summary per file
File Description
pkg/github/pullrequests.go Adds lockdown enforcement and fixes review filtering.
pkg/github/pullrequests_test.go Adds lockdown and empty-author coverage.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread pkg/github/pullrequests.go
Comment thread pkg/github/pullrequests.go Outdated
Comment thread pkg/github/pullrequests.go
@kerobbi kerobbi changed the title Apply lockdown check to pull_request_read get_diff/get_files/get_check_runs Apply lockdown check to pull_request_read get_diff/get_files Jul 16, 2026
@SamMorrowDrums
SamMorrowDrums merged commit 5995d96 into kerobbi/lockdown-shared-enforcement Jul 16, 2026
16 checks passed
@SamMorrowDrums
SamMorrowDrums deleted the kerobbi/lockdown-pr-read-paths branch July 16, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants